Repository navigation
feat(toolchain): outdated warnings for toolchain + agent skills, install skills from repo head - #3970
Merged
Merged
Conversation
…helpers New baml_release::skills module shared by the wrapper and baml-cli: skill repo URL builders (env-overridable for mirrors/tests), [skills] provenance state in state.toml written via read-modify-write so it never clobbers the wrapper-owned [channels.*] sections, a latest-commit freshness cache, and a GitHub API fetch for the skill repo head commit. Co-authored-by: Cursor <[email protected]>
…rovenance baml agent install now defaults to the current head of BoundaryML/baml-skill: it resolves the main commit SHA first, downloads the tarball pinned to that exact commit, and records [skills] provenance in state.toml (plus the freshness cache) after a successful install. The --latest flag is removed since it is the standard behavior; --from remains for custom sources and the toolchain-release snapshot survives only behind BAML_AGENT_SKILLS_RELEASE_VERSION. Includes an e2e test that runs the real binary against a stubbed GitHub server and asserts the recorded provenance. Co-authored-by: Cursor <[email protected]>
…ated The wrapper now surfaces staleness on normal commands: before exec'ing baml-cli it compares the recorded channel version against the cached channel manifest and the [skills] provenance against the cached latest skill commit, printing a styled bold-yellow warning: line on stderr when either is behind. Caches auto-refresh over the network at most once per 24h window (5s timeout, silent on failure, throttled by a .last-check marker so offline machines do not retry every command), activating the previously dead [update] auto_check config flag with default-on/opt-out semantics. toolchain status gains a skill section and toolchain use/update do a best-effort skill check afterward. The wrapper's State struct now round-trips foreign state.toml sections so it cannot drop the CLI-written [skills] entry. Covered by unit tests plus an e2e suite that runs the real wrapper binary against a temp BAML_HOME. Co-authored-by: Cursor <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe PR adds shared skill provenance and commit-cache APIs, changes skill installation to use exact repository commits, adds wrapper freshness checks and warnings with throttled refreshes, and removes packaged agent-skill release artifacts. ChangesSkills freshness lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BamlWrapper
participant SkillAPI
participant SkillCache
participant BamlCLI
User->>BamlWrapper: run command
BamlWrapper->>SkillCache: read cached freshness data
BamlWrapper->>SkillAPI: refresh stale skill commit
SkillAPI-->>SkillCache: store latest commit
BamlWrapper->>User: emit local freshness warning
BamlWrapper->>BamlCLI: forward command
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Projects without any baml-* agent skills now get a warning on every command (No baml skill is installed, set it up with baml agent install), instead of staying silent. Projects with skills keep the upgrade warning when provenance is missing or behind the cached latest commit; the project-skills check is now the primary gate, so global provenance no longer triggers upgrade warnings in skill-less projects. Decision logic extracted into a pure skill_warning_message helper with unit tests, plus updated e2e coverage. Co-authored-by: Cursor <[email protected]>
Binary size checks passed✅ 7 passed
Generated by |
CI runs clippy with -D warnings, which rejected the numeric casts in the skills date helper (cast_possible_wrap/truncation/sign_loss), a nonminimal_bool in file_older_than, and format_push_string in the e2e test. Replace the casts with try_from with in-range fallbacks and simplify the boolean with is_none_or. Co-authored-by: Cursor <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
baml_language/crates/baml_cli/tests/agent_install_e2e.rs (1)
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the exact resolved commit is downloaded.
The stub accepts
/archive/mainor any other ref, so a regression away from commit-pinned installation would still pass.Proposed test tightening
- } else if path.starts_with("/archive/") { + } else if path == format!("/archive/{COMMIT}") { ("application/gzip", skill_archive())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_cli/tests/agent_install_e2e.rs` around lines 54 - 60, Update the request handling in the agent-install test’s stub server so the `/archive/` branch asserts that the requested path contains the exact resolved commit returned by the `/commits` response, rather than accepting any ref such as `main`. Track or derive the expected commit and fail the test when the archive URL is not pinned to it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/crates/baml_cli/src/agent_command.rs`:
- Around line 87-89: The install flow must clear stale installed-commit
provenance when the loaded source has no commit, including --from and
release-snapshot paths. Update the logic surrounding loaded.commit and
record_installed_commit to remove or replace the existing installed_commit entry
whenever commit is None, while retaining normal recording for committed sources;
apply the same behavior across the related install branches.
In `@baml_language/crates/baml_release/src/skills.rs`:
- Around line 63-68: Update state handling around state_path() so skill
provenance is scoped to each project installation root rather than stored as one
global commit. Key provenance by the canonical install root, or place its state
file beside the project, and update the agent install/read/validation paths
consistently while preserving the wrapper-owned toolchain section.
- Around line 104-117: Make write_skills_state lossless and concurrency-safe:
only initialize an empty TOML document when reading state_path returns NotFound,
while propagating other read or parse failures instead of falling back to an
empty table. Add a cross-process lock around the entire read-modify-write
transaction, including parsing, updating the skills entry, serialization, and
atomic rename; apply the same locking and error-handling changes to the related
state update function around lines 167–175.
In `@baml_language/crates/baml/src/main.rs`:
- Around line 569-586: Use a single total timeout budget for the passive refresh
logic surrounding fetch_manifest_with_timeout and fetch_latest_skill_commit,
rather than allowing each serial request its own AUTO_CHECK_TIMEOUT. Establish
one deadline before the refreshes, calculate remaining time before each request,
and skip or bound any refresh when the budget is exhausted.
- Around line 678-685: Update dir_contains_baml_skill to count only valid skill
installations: inspect entries whose names start with “baml-”, verify each is a
directory, and require that it contains a SKILL.md file before returning true.
---
Nitpick comments:
In `@baml_language/crates/baml_cli/tests/agent_install_e2e.rs`:
- Around line 54-60: Update the request handling in the agent-install test’s
stub server so the `/archive/` branch asserts that the requested path contains
the exact resolved commit returned by the `/commits` response, rather than
accepting any ref such as `main`. Track or derive the expected commit and fail
the test when the archive URL is not pinned to it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b1b96241-8924-4c87-854e-5014ab8acd39
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
baml_language/crates/baml/Cargo.tomlbaml_language/crates/baml/src/main.rsbaml_language/crates/baml/tests/freshness_e2e.rsbaml_language/crates/baml_cli/src/agent_command.rsbaml_language/crates/baml_cli/tests/agent_install_e2e.rsbaml_language/crates/baml_release/Cargo.tomlbaml_language/crates/baml_release/src/lib.rsbaml_language/crates/baml_release/src/skills.rs
…se pipeline
agent install now pulls the skill repo head directly from GitHub, so the
versioned baml-agent-skills-{version}.tar.gz release asset has no
consumers. Remove the build-agent-skills job, the packaging script, and
the BAML_AGENT_SKILLS_RELEASE_VERSION escape hatch that pointed at it.
Co-authored-by: Cursor <[email protected]>
- agent install --from now clears recorded provenance instead of leaving a stale commit that misreports custom content as current - write_skills_state treats only a missing state.toml as empty; corrupt or unreadable files error instead of being silently replaced - passive cache refreshes share one 5s wall-clock budget instead of up to 5s each - skill detection requires a baml-* directory containing SKILL.md, so leftover empty directories don't suppress the install prompt Co-authored-by: Cursor <[email protected]>
…uto-check The passive path inlined its own fetch-then-persist sequence; route it through the same helper the explicit toolchain commands use so there is exactly one fetch-and-cache path and one compare-and-warn path, with trigger policy (TTL-gated vs forced) as the only difference. Co-authored-by: Cursor <[email protected]>
…ad of before it Warnings from existing caches still print immediately (no network). When a refresh is due (at most once per TTL window), it now runs on a background thread while baml-cli executes; the wrapper spawns and waits instead of exec'ing on that rare path, joins the refresh with whatever remains of the 5s budget after the command exits, and prints only warnings the fresh caches newly justify (never duplicating one already shown). The common no-refresh case keeps the zero-overhead exec path. Co-authored-by: Cursor <[email protected]>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml/tests/freshness_e2e.rs (1)
223-244: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a termination mechanism to the stub server thread.
The
incoming()loop runs indefinitely and theJoinHandleis dropped (detached), so the thread and itsTcpListenerpersist until process exit. For a single test this is harmless, but if more tests reuse this helper, detached listeners and threads can accumulate. AnAtomicBoolflag orstd::sync::mpscchannel to break the loop would allow clean shutdown.This is a test-only concern and can be deferred.
♻️ Optional: add a shutdown flag to the stub server
fn spawn_stub_commits_server(sha: &str) -> String { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let base = format!("http://{}", listener.local_addr().unwrap()); let body = format!(r#"{{"sha":"{sha}"}}"#); + let (tx, rx) = std::sync::mpsc::channel::<()>(); std::thread::spawn(move || { - for stream in listener.incoming() { + for stream in listener.incoming() { + if rx.try_recv().is_ok() { break; } let Ok(mut stream) = stream else { continue }; let mut buf = [0u8; 4096]; let _ = stream.read(&mut buf); let _ = stream.write_all( format!( "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", body.len() ) .as_bytes(), ); } }); + // Return (base_url, shutdown_sender) so callers can stop the server. base }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml/tests/freshness_e2e.rs` around lines 223 - 244, Make spawn_stub_commits_server return a shutdown handle alongside the base URL, and add an AtomicBool or mpsc channel checked by the listener loop so it can exit cleanly. Ensure the returned handle signals shutdown and joins the spawned thread, while preserving the existing request/response behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@baml_language/crates/baml/tests/freshness_e2e.rs`:
- Around line 223-244: Make spawn_stub_commits_server return a shutdown handle
alongside the base URL, and add an AtomicBool or mpsc channel checked by the
listener loop so it can exit cleanly. Ensure the returned handle signals
shutdown and joins the spawned thread, while preserving the existing
request/response behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a40976d-305c-4f53-b27f-90c003be91a0
📒 Files selected for processing (2)
baml_language/crates/baml/src/main.rsbaml_language/crates/baml/tests/freshness_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml/src/main.rs
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Jul 13, 2026
…lls, and baml-old_skills archive (BoundaryML#4001) ## Summary Moves the agent-skill freshness system wholly into the toolchain binary and rounds out the install flow: the "install/upgrade your skill" warning now ships with every nightly (no wrapper release needed), fires only on the authoring commands, and `baml agent install` archives whatever it replaces. ### New features - **Toolchain-owned skill warning** (`skill_check` in `baml_cli`): before `init`, `run`, `generate`, and `pack` — and only those — the CLI checks the project for installed `baml-*` skills and prints one of: - `warning: No baml skill is installed, set it up with baml agent install.` (no skills found between the cwd and `$HOME`) - `warning: Your baml skill is outdated, use baml agent install to upgrade it.` (installed provenance is behind the skill repo's latest commit) The check never blocks on the network: warnings are decided from local caches, and a background refresh of the latest-commit cache runs at most once per 24h (throttled by a marker even when offline, disabled via `[update] auto_check = false` in `~/.baml/config.toml`). All other commands — `check`, `describe`, `fmt`, `lsp`, `agent`, etc. — never nag. - **Replaced skills are archived, not deleted**: when `baml agent install` overwrites an existing skill, the previous version moves to `baml-old_skills/<name>/` inside the same skills directory (one slot per skill — each install keeps the immediately-previous copy). The archive doubles as the failure-recovery source. Archived copies nest one level deeper than the `skills/<name>/SKILL.md` layout, so agent harnesses don't discover them (verified against a live Claude Code session), the missing-skill detection ignores them, and `--from <dir>` re-ingestion filters them out. - **Wrapper cleanup**: all skill-warning machinery is removed from the `baml` wrapper (pass-through warning, post-`toolchain use/update` nudge, skill half of the lazy refresh, skill section of `toolchain status`). None of it had ever shipped — it was added in BoundaryML#3970 without a wrapper version bump — so published wrapper behavior is unchanged and the wrapper stays at 0.2.0. The shared logic (warning decision, project-skills walk, TTL/marker throttle, auto-check config gate) now lives in `baml_release::skills`. ### Expected behavior (first-run journey) Starting from an empty directory: ```console $ baml init warning: No baml skill is installed, set it up with baml agent install. Creating baml.toml (project) Creating baml_src/main.baml $ baml agent install Installed BAML agent skills in <project> Claude Code: .claude/skills/baml-*/SKILL.md Codex / OpenCode: .agents/skills/baml-*/SKILL.md ``` - The install downloads the skill repo's main-branch tarball from `codeload.github.com` — never the GitHub REST API (no credentials, no unauthenticated rate limit) — resolves the project root from the cwd (`baml.toml`/`baml_src` owner, bounded by the git repo / `$HOME`), and records the tarball's source commit as provenance in `~/.baml/state.toml`. - If a skill was already installed, the old version is kept at `.claude/skills/baml-old_skills/baml-core/` (and the `.agents` twin) while the new one replaces `baml-core/`. - Subsequent `run`/`generate`/`pack`/`init` are silent while the installed commit matches the repo head; once the skill repo advances (checked in the background ≤ once/day), they warn `Your baml skill is outdated…` until the next `baml agent install`. - Custom-source installs (`--from <url|tarball|dir>`) record no provenance, so they keep nudging toward an official install by design. ### Tests - New e2e suites: `skill_warning_e2e.rs` (11 tests: whitelist on/off, missing vs outdated vs current, background refresh persistence, offline throttling, auto-check opt-out, archive-not-installed) and the first-run journey in `agent_install_e2e.rs` (init warns → plain install from a codeload stub that 404s any API path → next command quiet). - Unit tests for the archive rotation (single slot keeps only the previous version) and the shared warning/throttle helpers in `baml_release`. - `exit_code_e2e.rs` now isolates `BAML_HOME` per test so CLI tests never read the developer's real `~/.baml` or touch the network. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added passive, cache-driven agent-skill install/upgrade warnings for select authoring commands, with TTL-throttled background refresh of the cached “latest skill commit”. - Updated agent skill installs to keep replaced versions in a single `baml-old_skills/` archive slot (per skill name) with rollback on failure. - **Bug Fixes** - Wrapper freshness scope narrowed to channel-manifest only; wrapper no longer computes or prints agent-skill freshness warnings. - **Documentation** - Clarified `update.auto_check` to describe wrapper manifest refresh only. - **Tests** - Revised and added e2e coverage for wrapper toolchain warnings, agent install/rollback, and passive skill warning behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Skill & toolchain freshness: outdated warnings, repo-head skill installs, and provenance tracking
Previously the toolchain never told users they were behind (no check, no nudge —
state.tomlstayed frozen until a manualbaml toolchain update), andbaml agent installinstalled the skill snapshot attached to the running toolchain version rather than the latest skills. This PR makes staleness visible for both, and makes skill installs track the skill repo head.What changed
1.
baml agent installinstalls the head ofBoundaryML/baml-skillmaincommit SHA via the GitHub API, then downloads the tarball pinned to that exact commit — so what gets recorded always matches what got installed.--latestis removed — it's the standard behavior now.--from <url|archive|dir>remains for custom sources.baml-agent-skills-{version}.tar.gz) is removed entirely: thebuild-agent-skillsrelease job, thescripts/package-agent-skillspackaging script, and theBAML_AGENT_SKILLS_RELEASE_VERSIONenv escape hatch are all gone. Skills are no longer versioned with or attached to toolchain releases.~/.baml/state.toml, mirroring how channel versions are recorded:[skills]section lives in a new sharedbaml_release::skillsmodule used by both the wrapper andbaml-cli; both writers use clobber-safe writes so neither can drop the other's sections.2. The wrapper warns when you're outdated (or have no skills at all)
On every normal command (
baml generate,baml run, …), before exec'ingbaml-cli, the wrapper compares local state against the freshness caches and prints to stderr (bold-yellowwarning:prefix, TTY-aware, matching existing CLI diagnostics):baml-*skills installed get the install prompt on every command, regardless of caches — so users discover skills exist.[skills].installed_commitis missing or differs from the cached latest skill-repo commit. The project-skills check is the primary gate: global provenance never triggers upgrade warnings in a project that has no skills (it gets the install prompt instead).3. Auto-refresh at a 24h cadence, hidden behind the command
.last-checkmarker throttles retries so an offline machine pays the cost once per day, not per command.baml-cliexecutes, not before it. Warnings from existing caches still print immediately (pure local reads). After the command exits, the wrapper waits out whatever remains of the 5s budget (a command that ran 3s waits at most 2s more) and prints only warnings the fresh caches newly justify — a warning already shown up front is never duplicated.baml-cliand waits (exec would kill the refresh thread); every other invocation keeps the zero-overhead Unixexecfast path.[update] auto_checkconfig flag: default on, opt out withauto_check = falsein~/.baml/config.toml(opting out disables the network refresh; warnings still compare against whatever explicit commands cached).4. Toolchain commands report skill status
baml toolchain statusgains a skill section (skill installed:/skill latest:/skill status:with an upgrade hint); network failure degrades to(failed to check)rather than failing the command.baml toolchain use/updatedo a best-effort skill freshness check afterward.Expected behavior summary
warning:on stderr on every command untiltoolchain updatebaml-*skillsagent installrunsagent installre-runsexecs straight intobaml-cliauto_check = falseagent installsucceedsNew env overrides (for mirrors/tests; never required)
BAML_AGENT_SKILLS_REPO,BAML_AGENT_SKILLS_COMMITS_URL,BAML_AGENT_SKILLS_ARCHIVE_BASE_URL— all default toBoundaryML/baml-skillon GitHub.Breaking / behavioral notes
baml agent install --latestno longer exists (its behavior is the default).BAML_AGENT_SKILLS_RELEASE_VERSIONescape hatch are removed; existing release assets on old tags are unaffected but new releases won't ship one.baml-agent-skills-{version}.tar.gz(thebuild-agent-skillsjob and packaging script are deleted fromrelease-baml-language.yml).auto_check).Testing
[channels.*]/[skills]can't clobber each other (including refusing to overwrite a corruptstate.toml),[skills]clearing for custom-source installs,auto_checksemantics, archive/URL handling.bamlbinary against a tempBAML_HOME, asserting warnings on stderr (install prompt vs upgrade warning, empty skill dirs don't count as installed), marker throttling with a dead endpoint, opt-out behavior, and the lazy-refresh path (background refresh persists the cache and surfaces the warning after the command, exactly once).agent installe2e test running the realbaml-cliagainst a stubbed local GitHub server, asserting installed skills + recorded provenance + cache update.toolchain statusshowed both sections, warning cleared after install;agent --help/agent install --helpverified to no longer mention--latest.cargo clippy --all-targets -- -D warningspasses locally for all three touched crates (the initial CI run flagged strict cast lints in the date helper plus two pedantic lints; fixed in the final commit withtry_from+ in-range fallbacks instead ofascasts).canary(merged in).Summary by CodeRabbit
New Features
agent installnow installs skills from the latest repository commit or a specified URL/path.Bug Fixes
Chores